Skip to content

Default uploads to curl with pinned headers#747

Merged
stevevanhooser merged 3 commits intomainfrom
claude/fix-linux-compression-uploads-JkWEX
Apr 18, 2026
Merged

Default uploads to curl with pinned headers#747
stevevanhooser merged 3 commits intomainfrom
claude/fix-linux-compression-uploads-JkWEX

Conversation

@stevevanhooser
Copy link
Copy Markdown
Contributor

Summary

  • Default ndi.cloud.api.files.putFiles (and the underlying PutFiles implementation class) to useCurl = true, matching the curl-by-default we already landed on downloads.
  • Pin upload headers in the curl path to Content-Type: application/octet-stream and Accept-Encoding: identity so the object metadata stored by S3 is identical no matter which client issued the PUT. Add -f so stale signed URLs (403/404) fail loudly instead of being reported as a successful upload.
  • Forward options.useCurl from uploadSingleFile's bulk-upload branch (previously only the non-bulk branch forwarded it, so bulk uploads silently used the MATLAB HTTP path).

Why

Audit of the upload paths showed several places still using MATLAB's native HTTP stack (zipForUpload, uploadDocumentCollection batch, uploadSingleFile bulk branch, anything calling putFiles without useCurl). Different clients tag S3 objects with different headers, which is the most plausible explanation for the flaky "Invalid TAR file" errors on freshly-uploaded .tgz artifacts — object metadata varies depending on which code path performed the upload. Standardizing on curl with pinned headers makes the stored metadata deterministic and matches the download side.

Test plan

  • Re-run ndi.unittest.cloud.readIngested on Linux and Mac.
  • Re-run ndi.symmetry.makeArtifacts.dataset.downloadIngested/testDownloadIngestedArtifacts on Linux.
  • Spot-check aws s3api head-object on a freshly-uploaded .tgz to confirm ContentType: application/octet-stream and no ContentEncoding metadata.

https://claude.ai/code/session_01HMnM1qnDBgGdjSqSnjTfsV

Pre-signed PUT uploads going through MATLAB's native HTTP client
sometimes store objects in S3 with headers that don't match what curl
sends, producing inconsistent Content-Encoding/Content-Type metadata
on the objects and flaky downloads later. Flip the default to curl for
consistency with ndi.cloud.api.files.getFile.
Align the implementation class with the wrapper's new curl-by-default
behavior. Explicitly set Content-Type: application/octet-stream and
Accept-Encoding: identity on the PUT so the metadata stored on the S3
object is consistent across clients. Add -f so stale signed URLs (403)
or missing objects (404) fail loudly instead of being reported as a
successful 200 OK upload.
The non-bulk branch already forwarded options.useCurl to putFiles, but
the bulk-upload branch called putFiles with no options, so it always
took the (previously false) default. With the default now true this is
cosmetic, but making the forward explicit keeps the caller's choice
honored if useCurl is ever set to false here.
@github-actions
Copy link
Copy Markdown
Contributor

Test Results

643 tests  ±0   643 ✅ +1   27m 54s ⏱️ -14s
117 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 39ac4ca. ± Comparison against base commit b483006.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.17%. Comparing base (a752e9e) to head (39ac4ca).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...+ndi/+cloud/+api/+implementation/+files/PutFiles.m 80.00% 1 Missing ⚠️
src/ndi/+ndi/+cloud/uploadSingleFile.m 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
- Coverage   32.24%   32.17%   -0.07%     
==========================================
  Files         671      671              
  Lines       29463    29466       +3     
==========================================
- Hits         9500     9481      -19     
- Misses      19963    19985      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stevevanhooser stevevanhooser merged commit a9cf5d3 into main Apr 18, 2026
5 of 6 checks passed
@stevevanhooser stevevanhooser deleted the claude/fix-linux-compression-uploads-JkWEX branch April 18, 2026 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants